feat(run): auto-convert UI-format workflows via server endpoint#448
Merged
Conversation
UI/save-format workflows (the default from ComfyUI's "Save" button) were rejected by `comfy run --workflow` with no path forward. POST them to the running server's /workflow/convert endpoint instead, and when the endpoint isn't installed print actionable guidance rather than a flat rejection. Signed-off-by: bigcat88 <bigcat88@icloud.com>
dosubot
Bot
added
size:M
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds UI-format detection, server-side conversion via POST /workflow/convert, integrates conversion into execute() (server check, detect, convert, validate), and extends tests for detection, conversion success/failure, and execute-time error handling — convert, then run, workflow's done in fun! ChangesUI Workflow Detection and Server-Side Conversion
Sequence DiagramsequenceDiagram
participant CLI as CLI:execute()
participant Server as ComfyUI_Server
participant Converter as /workflow/convert
participant Executor as WorkflowExecution
CLI->>Server: check server running
Server-->>CLI: running
CLI->>CLI: read workflow JSON
CLI->>CLI: is_ui_workflow()?
alt UI-format
CLI->>Converter: POST UI workflow JSON
Converter-->>CLI: returns API-format JSON
else API-format
CLI->>CLI: validate API workflow via _validate_api_workflow()
end
CLI->>Executor: execute API workflow
Executor-->>CLI: execution outcome
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #448 +/- ##
==========================================
+ Coverage 79.31% 79.62% +0.30%
==========================================
Files 35 35
Lines 4433 4485 +52
==========================================
+ Hits 3516 3571 +55
+ Misses 917 914 -3
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
comfy_cli/command/run.py (1)
27-36:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard malformed API payloads before first-node access (avoid a StopIteration surprise party).
load_api_workflow()can raise on{}or non-dict JSON instead of returningNone, which bypasses the intended friendly rejection flow.💡 Suggested fix
def load_api_workflow(file: str): with open(file, encoding="utf-8") as f: workflow = json.load(f) + if not isinstance(workflow, dict) or not workflow: + return None if is_ui_workflow(workflow): return None # Try validating the first entry to ensure it has a node class property node_id = next(iter(workflow)) node = workflow[node_id] - if "class_type" not in node: + if not isinstance(node, dict) or "class_type" not in node: return None return workflow🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@comfy_cli/command/run.py` around lines 27 - 36, The code attempts next(iter(workflow)) without guarding against empty or non-dict JSON; update load_api_workflow so after json.load(f) it first checks that workflow is a non-empty dict (e.g., if not isinstance(workflow, dict) or not workflow: return None) before calling is_ui_workflow or doing node_id = next(iter(workflow)); this prevents StopIteration or mis-handling of non-dict payloads while keeping the existing checks for "class_type" on the first node.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_cli/command/run.py`:
- Around line 55-66: The code currently calls json.loads(resp.read()) and can
crash if the /workflow/convert endpoint returns non-JSON; wrap the JSON parse in
a try/except that catches json.JSONDecodeError (and optionally ValueError) after
the successful request (where resp is returned from request.urlopen) and handle
it the same way as other conversion failures: pprint a user-friendly message via
pprint (include a brief snippet or note that response was not valid JSON), then
raise typer.Exit(code=1); update the block around json.loads(...) in run.py
(referencing json.loads, resp, pprint, and typer.Exit) to perform this graceful
exit on invalid JSON.
---
Outside diff comments:
In `@comfy_cli/command/run.py`:
- Around line 27-36: The code attempts next(iter(workflow)) without guarding
against empty or non-dict JSON; update load_api_workflow so after json.load(f)
it first checks that workflow is a non-empty dict (e.g., if not
isinstance(workflow, dict) or not workflow: return None) before calling
is_ui_workflow or doing node_id = next(iter(workflow)); this prevents
StopIteration or mis-handling of non-dict payloads while keeping the existing
checks for "class_type" on the first node.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5b5f2f19-15f7-4ecc-a02c-95c12639f4a5
📒 Files selected for processing (2)
comfy_cli/command/run.pytests/comfy_cli/command/test_run.py
Previously, a non-JSON 200 response (e.g. an HTML error page from a proxy in front of ComfyUI) would let json.JSONDecodeError escape uncaught and the CLI would dump a Python traceback. Translate it — plus non-object JSON payloads — into the same pprint + typer.Exit(1) pattern used by the other conversion failure paths. Signed-off-by: bigcat88 <bigcat88@icloud.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_cli/command/run.py`:
- Around line 101-102: Wrap the json.load call that reads workflow_name in a
try/except that catches json.JSONDecodeError, log or print a concise,
user-friendly error referencing the workflow file (workflow_name) and the decode
error message, and exit with a non-zero status instead of allowing the
traceback; update the block where raw_workflow = json.load(f) is executed so the
except uses json.JSONDecodeError and mirrors the friendly behavior used in the
/workflow/convert path.
- Around line 101-115: Refactor to avoid re-reading the file: modify
load_api_workflow to accept the already-parsed workflow dict (or add a new
validate_api_workflow(raw_workflow) that performs the API-workflow validation),
remove the redundant is_ui_workflow check inside that function, and update the
call in execute()/run.py to pass raw_workflow (not workflow_name) into the
validator/loader; retain the existing error path (pprint + raise typer.Exit)
when the validator indicates an invalid API workflow and keep
convert_ui_workflow_via_server(...) usage for the UI branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 30133aea-d7ce-4515-8986-ff75074d6ffd
📒 Files selected for processing (2)
comfy_cli/command/run.pytests/comfy_cli/command/test_run.py
Extract _validate_api_workflow as a sibling helper that operates on the
already-parsed dict. execute() now reads the file once into raw_workflow
and dispatches both the UI-conversion and API-validation branches off
that single parse, instead of re-opening and re-parsing in the non-UI
branch via load_api_workflow.
Side effect: _validate_api_workflow gains defensive checks for non-dict
and empty workflows, turning previously uncaught crashes (StopIteration
on next(iter({})), AttributeError when first entry isn't a dict) into
clean "not an API workflow" rejections.
Signed-off-by: bigcat88 <bigcat88@icloud.com>
Previously `json.load(f)` in execute() would let JSONDecodeError escape uncaught, dumping a Python traceback. Translate it into the same friendly pprint + typer.Exit(1) pattern used by the /workflow/convert path so both file-side and server-side parse failures behave consistently. Signed-off-by: bigcat88 <bigcat88@icloud.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_cli/command/run.py`:
- Around line 111-122: When is_ui_workflow(raw_workflow) is true and you call
convert_ui_workflow_via_server(...), validate the returned workflow with
_validate_api_workflow before proceeding: after convert_ui_workflow_via_server
returns, pass its result into _validate_api_workflow and if it returns falsy,
call the same error path used in the else branch (pprint the "[bold
red]Specified workflow does not appear to be an API workflow json file[/bold
red]" message and raise typer.Exit(code=1)); keep the existing
WorkflowConverterUnavailable handling via _print_converter_unavailable_help.
- Around line 104-110: The file-open/JSON load block (open(workflow_name,
encoding="utf-8") and json.load into raw_workflow) can raise OSError (e.g.,
permission denied) as well as json.JSONDecodeError; update the exception
handling so OSError is caught and handled the same way as JSON errors — show the
friendly pprint message and raise typer.Exit(code=1) from the caught exception
(either by adding OSError to the existing except tuple or adding a separate
except OSError block) to ensure consistent CLI behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7056e96-6218-43cb-8ce3-875e030e758e
📒 Files selected for processing (2)
comfy_cli/command/run.pytests/comfy_cli/command/test_run.py
open() can raise OSError (PermissionError, FileNotFoundError on a between-isfile-and-open race, etc.) even after os.path.isfile() returns true. Catch it alongside the JSONDecodeError handler so a permission denied or transient filesystem failure prints a one-line message instead of dumping a Python traceback. Signed-off-by: bigcat88 <bigcat88@icloud.com>
dosubot
Bot
added
size:L
There was a problem hiding this comment.
♻️ Duplicate comments (1)
comfy_cli/command/run.py (1)
114-120:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate the converted payload before execution.
convert_ui_workflow_via_server()only guarantees “JSON object”, not “API workflow”. If the server/plugin returns a dict that still lacksclass_type, this branch hands it toWorkflowExecutionand the failure shows up later as a muddier/prompterror — bad payloads shouldn’t get to jig downstream.♻️ Tight patch
if is_ui_workflow(raw_workflow): pprint("[yellow]Detected UI-format workflow, converting via server's /workflow/convert...[/yellow]") try: - workflow = convert_ui_workflow_via_server(raw_workflow, host, port, timeout) - except WorkflowConverterUnavailable: + converted = convert_ui_workflow_via_server(raw_workflow, host, port, timeout) + except WorkflowConverterUnavailable as exc: _print_converter_unavailable_help() - raise typer.Exit(code=1) + raise typer.Exit(code=1) from exc + workflow = _validate_api_workflow(converted) + if not workflow: + pprint("[bold red]Workflow conversion failed: server returned an invalid API workflow[/bold red]") + raise typer.Exit(code=1) else: workflow = _validate_api_workflow(raw_workflow) if not workflow: pprint("[bold red]Specified workflow does not appear to be an API workflow json file[/bold red]") raise typer.Exit(code=1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@comfy_cli/command/run.py` around lines 114 - 120, After converting via convert_ui_workflow_via_server in the is_ui_workflow branch, validate that the returned workflow is a mapping and contains the required API key 'class_type' before handing it to WorkflowExecution; if the converted payload is not a dict or is missing 'class_type', call _print_converter_unavailable_help() or a clear error log and raise typer.Exit(code=1) (same exit flow as WorkflowConverterUnavailable) so bad payloads never reach WorkflowExecution. Use the existing symbols is_ui_workflow, convert_ui_workflow_via_server, _print_converter_unavailable_help, and WorkflowExecution to locate where to insert the check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@comfy_cli/command/run.py`:
- Around line 114-120: After converting via convert_ui_workflow_via_server in
the is_ui_workflow branch, validate that the returned workflow is a mapping and
contains the required API key 'class_type' before handing it to
WorkflowExecution; if the converted payload is not a dict or is missing
'class_type', call _print_converter_unavailable_help() or a clear error log and
raise typer.Exit(code=1) (same exit flow as WorkflowConverterUnavailable) so bad
payloads never reach WorkflowExecution. Use the existing symbols is_ui_workflow,
convert_ui_workflow_via_server, _print_converter_unavailable_help, and
WorkflowExecution to locate where to insert the check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5f00d36d-c925-4955-901d-abcf4ef39ad3
📒 Files selected for processing (2)
comfy_cli/command/run.pytests/comfy_cli/command/test_run.py
Catches the cases where /workflow/convert returns a valid JSON object
that still isn't an executable workflow: empty dict, or a dict whose
first entry isn't a node dict with class_type. Without these, an error
response like {} or {"1": "not a node"} traveled downstream and either
produced a confusing /prompt 400 or crashed on string-indexing in the
WorkflowExecution title helpers.
Validation lives at the conversion boundary rather than reusing
_validate_api_workflow — the converter is an external API and its
output deserves a dedicated error message attributable to it.
Signed-off-by: bigcat88 <bigcat88@icloud.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_cli/command/run.py`:
- Around line 26-33: The _validate_api_workflow function currently only checks
the first top-level node; change it to iterate over all top-level values in the
workflow dict and verify each value is a dict containing the "class_type" key,
returning None if any fail and returning the workflow only if all pass; extract
the per-node check into a small helper (e.g., _is_api_node) and reuse that
helper in both _validate_api_workflow and the other validator referenced around
the 73-79 block so both validators consistently validate every node rather than
only next(iter(...)).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc699747-5348-4fad-b4d5-b5e859b66564
📒 Files selected for processing (2)
comfy_cli/command/run.pytests/comfy_cli/command/test_run.py
is_ui_workflow now requires `nodes` and `links` to actually be lists (matching the LiteGraph schema), so it can't mis-classify an API workflow whose node IDs happen to be the strings "nodes" or "links". Also corrects the workaround message to point at the new frontend menubar item — File > Export (API), which is not gated by Dev Mode — instead of the older "Save (API Format)" path that referenced an outdated UX. Signed-off-by: bigcat88 <bigcat88@icloud.com>
…sage * execute() now reads and parses the workflow file inline, so the load_api_workflow helper had no remaining callers; drop it along with its unit tests. * When /workflow/convert is missing, the user has already seen "Detected UI-format workflow, converting via server's /workflow/convert..."; the fallback no longer repeats that detection and goes straight to the cause. Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
comfy run --workflownow detects UI/save-format workflows (the default from ComfyUI's "Save" button) and POSTs them to the running server's/workflow/convertendpoint, then runs the converted API-format result."Specified workflow does not appear to be an API workflow json file"rejection.comfy run --workflow(auto-convert to API format) #446. A follow-up PR will add a client-side converter (using/object_info) so users don't need to install anything server-side.What changed in
comfy run --workflow/workflow/convertclass_type, not UI)The hint when the endpoint is missing:
Why this split
Part 2 (client-side converter) is ~1400 LOC of node-graph traversal — subgraphs, reroutes, bypassed nodes, dynamic combos, widget mapping via
/object_info. Landing the server-endpoint path first keeps that review independent, and the contract here (is_ui_workflow,convert_ui_workflow_via_server,WorkflowConverterUnavailable, the hint text) is what Part 2 will plug into as a fallback.Test plan
pytest tests/comfy_cli/command/test_run.py— 28/28 pass (12 new)pytest tests/comfy_cli/— 785/785 unit tests pass, no regressionsruff check+ruff format --checkclean/prompt)New tests
TestIsUiWorkflow— 4 tests for the format detectorTestConvertUiWorkflowViaServer— success, missing endpoint (404 + 405 parametrized), HTTP 500, network errorTestExecuteUiWorkflow— end-to-end conversion + execution, exits on missing endpoint, exits when server unreachable without attempting conversionTestExecuteErrorHandling::test_rejects_invalid_workflow_format— preserves the malformed-input rejectionRefs #446